Skip to content

feat: support scalar qjson.decode roots#122

Merged
membphis merged 1 commit into
mainfrom
codex/120-top-level-scalar-decode
May 31, 2026
Merged

feat: support scalar qjson.decode roots#122
membphis merged 1 commit into
mainfrom
codex/120-top-level-scalar-decode

Conversation

@membphis
Copy link
Copy Markdown
Collaborator

@membphis membphis commented May 31, 2026 •
edited by coderabbitai Bot
Loading

Summary

  • Allow qjson.decode() to return top-level scalar JSON roots via the existing cursor decode path.
  • Add Lua regression coverage for number, string, boolean, null, and invalid/empty decode inputs.

Fixes #120

Test Plan

  • DYLD_LIBRARY_PATH=./target/release LUA_PATH='./lua/?.lua;;' LUA_CPATH='./vendor/lua-cjson/?.so;./target/release/lib?.dylib;./target/release/lib?.so;./?.so;;' /opt/homebrew/bin/luajit -e 'local qjson=require("qjson"); assert(qjson.decode("42") == 42); assert(qjson.decode("3.14") == 3.14); assert(qjson.decode("true") == true); assert(qjson.decode("false") == false); assert(qjson.decode("null") == qjson.null); assert(qjson.decode("\"hello\"") == "hello"); local ok = pcall(qjson.decode, ""); assert(not ok); print("focused-ok")'
  • PATH=/Users/yuanshengwang/.luarocks/bin:$PATH make test
  • make lint
  • cargo test --release --no-default-features
  • cargo test --features test-panic --release

Summary by CodeRabbit

  • New Features

    • decode() now supports top-level JSON scalar values (numbers, strings, booleans, and null) in addition to objects and arrays.

  • Tests

    • Added comprehensive tests for top-level scalar value decoding scenarios.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 31, 2026 •
edited
Loading

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 1660cf09-f7d1-45a6-953a-0f8465657a09

📥 Commits

Reviewing files that changed from the base of the PR and between f4a4f70 and 899014e.

📒 Files selected for processing (2)
  • lua/qjson/table.lua
  • tests/lua/lazy_table_spec.lua
📝 Walkthrough

Walkthrough

The PR extends qjson.decode() to accept top-level JSON scalar values (numbers, strings, booleans, null) instead of raising an error. The implementation reuses the existing lazy cursor and view infrastructure to wrap scalar roots, while comprehensive test coverage validates the new decode behavior for all primitive types and error conditions.

Changes

Top-level scalar decode support

Layer / File(s) Summary
Scalar decode implementation and tests
lua/qjson/table.lua, tests/lua/lazy_table_spec.lua		 _M.decode() now returns top-level scalar values via decode_cursor(view, root_box) instead of raising an error. Test suite validates correct decoding of numbers, strings (with escapes), booleans, and null, plus error behavior on invalid/empty input.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

🚥 Pre-merge checks | ✅ 6
✅ Passed checks (6 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely summarizes the main change: enabling qjson.decode() to support scalar (non-object/array) JSON roots.
Linked Issues check ✅ Passed The PR implements all core requirements from issue #120: modified qjson.decode() to accept top-level scalars (numbers, strings, booleans, null), preserve existing behavior for objects/arrays and errors, and added comprehensive test coverage.
Out of Scope Changes check ✅ Passed All changes are directly scoped to the stated objectives: one-line modification in lua/qjson/table.lua to handle scalar roots and new test cases in lazy_table_spec.lua for validation.
E2e Test Quality Review ✅ Passed PR correctly implements scalar decode support and tests comprehensively cover all required types, edge cases, and error conditions with proper error handling.
Security Check ✅ Passed Change reuses safe decode_cursor() for top-level scalars. No sensitive data exposure, logging, database, authorization, or crypto concerns. Library scope makes categories 2-7 inapplicable.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/120-top-level-scalar-decode

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@membphis membphis merged commit 9d7a9f3 into main May 31, 2026
15 checks passed
@membphis membphis deleted the codex/120-top-level-scalar-decode branch May 31, 2026 14:37
@coderabbitai coderabbitai Bot mentioned this pull request Jun 1, 2026
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

F1: Support top-level scalar decode in qjson.decode()

1 participant

@membphis